Skip perf counter tests when counters are unavailable#2204
Conversation
| if (!benchmark::internal::PerfCounters::kSupported) { | ||
| return false; | ||
| } | ||
| benchmark::internal::PerfCounters::Initialize(); |
There was a problem hiding this comment.
Does PerfCounters::Create() not do this internally?
There was a problem hiding this comment.
You're right, Create() already calls Initialize() when the counter list is non-empty. I removed the extra call in 315b6db and left the helper as just an availability check.
| } | ||
|
|
||
| static bool HasUniqueCounterNames(const PerfCounters& pc, | ||
| const std::set<std::string>& names) { |
There was a problem hiding this comment.
I think this file should do the same as the other one,
the current fix looks a bit too unrefined i think.
There was a problem hiding this comment.
Good point. I replaced the repeated per-test checks with a shared HasRequiredPerfCounters helper in de12d46, so this file now uses the same preflight pattern as perf_counters_test.cc.
There was a problem hiding this comment.
I'm not sure there is a point in keeping HasUniqueCounterNames()?
Also, HasRequiredPerfCounters() should probably be shared,
at least via a header file.
|
|
||
| TEST(PerfCountersTest, OneCounter) { | ||
| if (!HasRequiredPerfCounters({kGenericPerfEvent1})) { | ||
| GTEST_SKIP() << "Requested performance counters are not available.\n"; |
There was a problem hiding this comment.
I don't think GTEST_SKIP needs a trailing newline.
There was a problem hiding this comment.
Done in b4c1ba3. I removed the trailing newlines from the GTEST_SKIP messages in this file.
|
Ok, this is going to be annoying. Let's not bother with sharing the function then :) |
|
Done, I removed the shared helper header in 23b83d0 and kept the availability checks local to each perf counter test file. That should avoid the Bazel include issue as well. Local validation:
|
LebedevRI
left a comment
LebedevRI
left a comment
There was a problem hiding this comment.
Looks reasonable-ish i guess.
3 participants
Fixes #2174
This handles platforms where benchmark is built with libpfm, but the specific counters used by these tests cannot be opened. In that case
PerfCounters::kSupportedis true, whilePerfCounters::Create()returns no matching counters.Changes:
perf_counters_testexit early unless bothCYCLESandINSTRUCTIONScan be createdNegativeTestintactTested:
docker run --rm -v "$PWD":/src -w /src ubuntu:22.04 ... ctest --test-dir build-linux -R "perf_counters_(gtest|test)$" --output-on-failure